Register JNI natives via blittable JniNativeMethod#1474
Open
simonrozsival wants to merge 4 commits into
Open
Conversation
The default native-method registration funnel invoked the JNI RegisterNatives function pointer typed as `delegate* unmanaged<nint, nint, JniNativeMethodRegistration[], int, int>`, passing a non-blittable managed array and relying on a runtime-synthesized marshalling stub to convert it to JNINativeMethod*. crossgen2 miscompiles that stub under composite ReadyToRun + PGO (MIBC): it degrades to a raw struct blit, so the native `name`/`signature` pointers end up referencing the managed string objects instead of marshalled UTF-8 data. The registered method names are corrupted, producing NoSuchMethodError at startup (e.g. MauiApplication, net.dot.jni.ManagedPeer). Marshal JniNativeMethodRegistration[] into blittable JniNativeMethod values and dispatch to the existing RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>) overload, eliminating the non-blittable `delegate* unmanaged<>` call site. This matches the trimmable type-map path, which was already immune. Fixes dotnet/android#11633 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR review: the generated `_RegisterNatives` wrapper checked `ExceptionOccurred()` after the native call and rethrew/cleared any pending Java exception (e.g. NoSuchMethodError). Routing registration through the blittable `RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>)` overload dropped that check, which could leave a pending exception in the JNIEnv and cause subsequent JNI calls to fail or hide the real error. Add the pending-exception check (and a `type.IsValid` guard) to the blittable overload so both the array-based registration path and the trimmable type-map path surface JNI registration failures correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates Java.Interop’s JNI native-method registration to avoid passing a non-blittable JniNativeMethodRegistration[] through a delegate* unmanaged<> call path, instead marshalling into blittable JniNativeMethod entries and using the existing RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>) overload. It also improves AOT analyzer signaling for APIs that rely on dynamic code and clarifies test suppressions.
Changes:
- Reworked
JniEnvironment.Types.RegisterNatives(JniObjectReference, JniNativeMethodRegistration[], int)to marshal names/signatures to unmanaged UTF-8 and dispatch via the blittable overload. - Added pending-Java-exception surfacing/clearing to the blittable
RegisterNativesoverload and addedRequiresDynamicCode/IL3050suppressions where appropriate. - Updated tests and
ManagedPeerto suppress IL3050 where they intentionally exercise non-AOT-compatible registration APIs.
Show a summary per file
| File | Description |
|---|---|
| tests/Java.Interop-Tests/Java.Interop/JniTypeTest.cs | Adds IL3050 suppressions for tests that intentionally call non-AOT-compatible native registration APIs. |
| src/Java.Interop/Java.Interop/ManagedPeer.cs | Suppresses IL3050 on ManagedPeer static ctor due to non-AOT-compatible native registration usage. |
| src/Java.Interop/Java.Interop/JniType.cs | Marks RegisterNativeMethods(JniNativeMethodRegistration[]) as RequiresDynamicCode for clearer AOT analyzer behavior. |
| src/Java.Interop/Java.Interop/JniEnvironment.Types.cs | Implements marshalling to blittable JniNativeMethod and improves exception handling for RegisterNatives. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 2
Comment on lines
+292
to
+312
| const int MaxStackAllocatedNativeMethods = 32; | ||
| bool useStackAllocatedBuffers = numMethods <= MaxStackAllocatedNativeMethods; | ||
| Span<JniNativeMethod> natives = useStackAllocatedBuffers | ||
| ? stackalloc JniNativeMethod [numMethods] | ||
| : new JniNativeMethod [numMethods]; | ||
| Span<IntPtr> unmanagedStrings = useStackAllocatedBuffers | ||
| ? stackalloc IntPtr [numMethods * 2] | ||
| : new IntPtr [numMethods * 2]; | ||
| try { | ||
| for (int i = 0; i < numMethods; ++i) { | ||
| var m = methods [i]; | ||
| IntPtr name = Marshal.StringToCoTaskMemUTF8 (m.Name); | ||
| IntPtr sig = Marshal.StringToCoTaskMemUTF8 (m.Signature); | ||
| unmanagedStrings [i * 2] = name; | ||
| unmanagedStrings [i * 2 + 1] = sig; | ||
| natives [i] = new JniNativeMethod ((byte*) name, (byte*) sig, GetFunctionPointerForDelegate (m.Marshaler)); | ||
| } | ||
| RegisterNatives (type, natives); | ||
| // Keep the Marshaler delegates alive at least until JNI has consumed the function pointers. | ||
| GC.KeepAlive (methods); | ||
| } finally { |
Comment on lines
+346
to
+349
| // Surface (and clear) any pending Java exception raised by JNI::RegisterNatives() | ||
| // — e.g. NoSuchMethodError — before falling back to the return-code check, matching | ||
| // the behavior of the generated `_RegisterNatives` wrapper. Leaving a pending | ||
| // exception in the JNIEnv would make subsequent JNI calls fail or abort. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebases the targeted native-registration fix from #1455 onto current Java.Interop for dotnet/android#11477.\n\nThis avoids passing the non-blittable JniNativeMethodRegistration[] overload through RegisterNatives; dynamic registrations such as dotnet/android's Debug TestInstrumentation n_onCreate now marshal names/signatures through blittable JniNativeMethod entries instead.